[PIX] Recompute shader flags after NURI instrumentation by damyanp · Pull Request #8846 · microsoft/DirectXShaderCompiler

Damyan Pepper (damyanp) · 2026-08-27T23:35:37Z

Part 5 of 14 in the PIX instrumentation stack. It targets users/damyanp/pix-fixes-04. Its content depends on PR 3, which changes the same pass and the same cleanup path.

The non-uniform resource index pass inserts WaveActiveAllEqual to test whether a dynamic index is uniform. It inserts these calls after the compiler computed the shader flags. The module can therefore declare no wave operations and still contain them, and the validator refuses that module.

The pass computes the flags again, but only when it changed the module.

Assisted-by: Copilot

This changes only the PIX instrumentation, so it needs no release note.

_{Stack created with GitHub Stacks CLI • Give Feedback 💬}

Copilot

Pull request overview

Recomputes DXIL shader flags after PIX NURI instrumentation inserts wave operations.

Changes:

Recollects shader flags before metadata re-emission.
Adds regression validation for the WaveOps flag.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File	Description
`lib/DxilPIXPasses/DxilNonUniformResourceIndexInstrumentation.cpp`	Recomputes shader flags after instrumentation.
`tools/clang/unittests/HLSL/PixTest.cpp`	Tests instrumented module validation and wave-op insertion.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

  VERIFY_IS_FALSE(IsPermittedValidationException(realDiagnostic));
 }
+
+TEST_F(PixTest, Validation_NonUniformResourceIndex_WaveOpsFlag) {


Copilot

Copilot review overview

🟢 Approval recommended

The implementation and regression coverage address the invalid shader flags, with only minor style feedback remaining.

Review effort: Balanced
Findings: 1 · 1

Open (2)

This test targets shader model 6.6 (DXIL 1.6) but does not guard for the loaded compiler/validator…
The repository's “almost never auto” convention calls for an explicit type here because Compile…

Copilot

Copilot review overview

🔵 Needs a closer look

The stated shader-flag recomputation is unchanged context, so the intended functional fix is absent from this diff.

Review effort: Balanced
Findings: 1 · 1

Open (2)

This test targets shader model 6.6 (DXIL 1.6) but does not guard for the loaded compiler/validator…
The repository's “almost never auto” convention calls for an explicit type here because Compile…

Previously missed (1)

In code that hasn't changed since last review

Diff omits the stated shader flag recomputation change

lib/DxilPIXPasses/DxilNonUniformResourceIndexInstrumentation.cpp:164

The described behavior change is not present in this diff: CollectShaderFlagsForModule() on the following line is unchanged context, so this hunk only adds a comment around behavior already in the target. Please update/rebase the stack so the actual recomputation change is included in this PR (or revise the PR scope if the implementation belongs to an earlier stack entry); otherwise merging this PR cannot deliver the stated fix.

Copilot

Copilot review overview

🔵 Needs a closer look

The new DXIL 1.6 test needs the established version guard to avoid failures with older validators.

Review effort: Balanced
Findings: 1 · 1

Open (2)

This test targets shader model 6.6 (DXIL 1.6) but does not guard for the loaded compiler/validator…
The repository's “almost never auto” convention calls for an explicit type here because Compile…

Copilot

Copilot review overview

🔵 Needs a closer look

The new test unconditionally requires DXIL 1.6 and can fail with older supported compiler or validator configurations.

Review effort: Balanced
Findings: 1 · 1

Open (2)

This test targets shader model 6.6 (DXIL 1.6) but does not guard for the loaded compiler/validator…
The repository's “almost never auto” convention calls for an explicit type here because Compile…

The non-uniform resource index pass inserts WaveActiveAllEqual after the shader flags were computed, so the module could contain wave operations it does not declare and fail validation. Recompute the flags when the pass changes the module. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 40dc9de3-617e-4caf-ab0d-fba0a033ed93

Copilot

Copilot review overview

🟢 Approval recommended

The implementation correctly updates emitted flags and includes focused regression coverage.

Review effort: Balanced
Findings: 1

Open (1)

This test targets shader model 6.6 (DXIL 1.6) but does not guard for the loaded compiler/validator…

Resolved since last review (1)

The repository's “almost never auto” convention calls for an explicit type here because Compile…

github-project-automation Bot added this to HLSL Roadmap Aug 27, 2026

github-project-automation Bot moved this to New in HLSL Roadmap Aug 27, 2026

Damyan Pepper (damyanp) requested a review from Austin Kinross (austinkinross) August 27, 2026 23:35

Damyan Pepper (damyanp) marked this pull request as ready for review August 27, 2026 23:37

Copilot AI balanced review requested due to automatic review settings August 27, 2026 23:37

Copilot started reviewing on behalf of Damyan Pepper (damyanp) August 27, 2026 23:37 View session

Austin Kinross (austinkinross) approved these changes Aug 27, 2026

View reviewed changes

Copilot AI reviewed Aug 27, 2026

View reviewed changes

Comment thread tools/clang/unittests/HLSL/PixTest.cpp Outdated

Comment thread tools/clang/unittests/HLSL/PixTest.cpp

VERIFY_IS_FALSE(IsPermittedValidationException(realDiagnostic));

}

TEST_F(PixTest, Validation_NonUniformResourceIndex_WaveOpsFlag) {

Copilot AI review requested due to automatic review settings September 29, 2026 02:06

Copilot started reviewing on behalf of Damyan Pepper (damyanp) September 29, 2026 02:07 View session

Copilot AI reviewed Sep 29, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings September 29, 2026 16:25

Copilot started reviewing on behalf of Damyan Pepper (damyanp) September 29, 2026 16:25 View session

Copilot AI reviewed Sep 29, 2026

View reviewed changes

Copilot AI balanced review requested due to automatic review settings September 30, 2026 00:02

Copilot started reviewing on behalf of Damyan Pepper (damyanp) September 30, 2026 00:03 View session

Copilot AI reviewed Sep 30, 2026

View reviewed changes

Base automatically changed from users/damyanp/pix-fixes-04 to main October 1, 2026 21:46

Copilot AI balanced review requested due to automatic review settings October 1, 2026 21:56

Damyan Pepper (damyanp) force-pushed the users/damyanp/pix-fixes-05 branch from 409fa0e to e76a6d7 Compare October 1, 2026 21:56

Copilot started reviewing on behalf of Damyan Pepper (damyanp) October 1, 2026 21:58 View session

Copilot AI reviewed Oct 1, 2026

View reviewed changes

Copilot AI balanced review requested due to automatic review settings October 1, 2026 22:57

Damyan Pepper (damyanp) force-pushed the users/damyanp/pix-fixes-05 branch from e76a6d7 to 2a293ba Compare October 1, 2026 22:57

Copilot started reviewing on behalf of Damyan Pepper (damyanp) October 1, 2026 22:57 View session

Copilot AI reviewed Oct 1, 2026

View reviewed changes

Navigation Menu

[PIX] Recompute shader flags after NURI instrumentation - #8846

[PIX] Recompute shader flags after NURI instrumentation#8846
Damyan Pepper (damyanp) wants to merge 1 commit into
mainfrom
users/damyanp/pix-fixes-05

Damyan Pepper (damyanp) commented Aug 27, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

Damyan Pepper (damyanp) commented Aug 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟢 Approval recommended

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🔵 Needs a closer look

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🔵 Needs a closer look

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🔵 Needs a closer look

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟢 Approval recommended

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Damyan Pepper (damyanp) commented Aug 27, 2026 •

edited

Loading